Skip to content

Fix/detached forecasting sensors#2035

Open
BelhsanHmida wants to merge 58 commits into
mainfrom
fix/detached-forecasting-sensors
Open

Fix/detached forecasting sensors#2035
BelhsanHmida wants to merge 58 commits into
mainfrom
fix/detached-forecasting-sensors

Conversation

@BelhsanHmida

@BelhsanHmida BelhsanHmida commented Mar 18, 2026

Copy link
Copy Markdown
Contributor

Description

Previously, train/predict cycle jobs enqueued a bound TrainPredictPipeline method, which caused the pipeline instance and its ORM-backed sensors/data source to be pickled and sent across the web-worker boundary. In the worker, those objects could be detached from the active DB session and fail with DetachedInstanceError, especially for regressors.

This PR changes cycle jobs to pass only serialized config/parameters plus data_source_id, then reconstructs the pipeline inside the worker. This keeps ORM objects session-local to the worker and avoids carrying session-bound state through RQ.

  • Serialize queued train-predict cycle jobs with plain IDs and timing/config payloads.
  • Reconstruct target sensor, save sensor, regressors, and data source inside the worker DB session.
  • Commit the forecasting DataSource before enqueuing jobs so workers can load it by ID.
  • Keep queued cycle jobs limited to serialized pipeline state and per-cycle timings; arbitrary job_kwargs are intentionally not forwarded.
  • Add regression coverage proving queued job kwargs do not contain ORM instances.
  • Add changelog entry.

Closes #2256

How to test

uv run pytest flexmeasures/data/tests/test_forecasting_pipeline.py flexmeasures/data/tests/test_forecasting_jobs.py flexmeasures/data/tests/test_forecasting_jobs_fresh_db.py flexmeasures/api/v3_0/tests/test_forecasting_api_fresh_db.py flexmeasures/data/tests/test_job_cache.py -q

Sign-off

  • I agree to contribute to the project under Apache 2 License.
  • To the best of my knowledge, the proposed patch is not based on code under GPL or other license that is incompatible with FlexMeasures

BelhsanHmida and others added 30 commits March 9, 2026 10:37
Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
Signed-off-by: F.N. Claessen <claessen@seita.nl>
Signed-off-by: F.N. Claessen <claessen@seita.nl>
Signed-off-by: F.N. Claessen <claessen@seita.nl>
Signed-off-by: F.N. Claessen <claessen@seita.nl>
…dated

Signed-off-by: F.N. Claessen <claessen@seita.nl>
…ters

Signed-off-by: F.N. Claessen <claessen@seita.nl>
…ycle job

Signed-off-by: F.N. Claessen <claessen@seita.nl>
Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
… pipeline

Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
…ecasts

Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
…rity

Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
…n.commit() some time to finish writing a new source to the db

Signed-off-by: F.N. Claessen <claessen@seita.nl>
Signed-off-by: F.N. Claessen <claessen@seita.nl>
This reverts commit 888b980.

Signed-off-by: F.N. Claessen <claessen@seita.nl>
Signed-off-by: F.N. Claessen <claessen@seita.nl>
Signed-off-by: F.N. Claessen <claessen@seita.nl>
Signed-off-by: F.N. Claessen <claessen@seita.nl>
@BelhsanHmida BelhsanHmida marked this pull request as ready for review June 1, 2026 11:44

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Comment thread flexmeasures/data/models/forecasting/pipelines/train_predict.py
BelhsanHmida and others added 2 commits July 6, 2026 09:28
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Mohamed Belhsan Hmida <149331360+BelhsanHmida@users.noreply.github.com>
Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
@BelhsanHmida BelhsanHmida requested a review from Flix6x July 6, 2026 22:46

@Flix6x Flix6x left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move the changelog entry to v1. Can you verify that the forecasting job page correctly opens in rq-dashboard to show the job kwargs?

Comment thread documentation/changelog.rst Outdated
Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
@BelhsanHmida

Copy link
Copy Markdown
Contributor Author

Please move the changelog entry to v1. Can you verify that the forecasting job page correctly opens in rq-dashboard to show the job kwargs?

Done, I moved the changelog entry to the v1 section.

I also verified that the forecasting job page opens correctly in rq-dashboard and shows the job kwargs:

image

The most recent job in the screenshot was created from this branch. The earlier jobs were created from main.

Main differences:

  • On main, the queued job calls the bound run_cycle(...) method directly.

  • On this branch, the queued job calls the module-level run_train_predict_cycle_job(...).

  • On main, most values are passed as flat kwargs to run_cycle(...), including ORM-backed objects like sensor=<Sensor ...> and sensor_to_save=<Sensor ...>.

  • On this branch, the kwargs are grouped into a safer structure:

    • config: model/config values, with regressors represented as future_regressor_ids / past_regressor_ids
    • parameters: forecast parameters, with sensors represented as sensor_id / sensor_to_save_id
    • data_source_id: the saved forecasting data source
    • cycle timing kwargs: train_start, train_end, predict_start, predict_end, counter, multiplier

So the same forecasting information is still visible in rq-dashboard, but ORM objects are replaced by IDs and grouped under config / parameters.

@BelhsanHmida BelhsanHmida requested a review from Flix6x July 7, 2026 23:09
@Flix6x

Flix6x commented Jul 8, 2026

Copy link
Copy Markdown
Member

The image is of the rq-dashboard jobs (plural) page. Can you click on the job ID to open the job (singular) page?

@BelhsanHmida

Copy link
Copy Markdown
Contributor Author

The image is of the rq-dashboard jobs (plural) page. Can you click on the job ID to open the job (singular) page?

  • Main:
Screenshot from 2026-07-08 12-42-18

This PR:
Screenshot from 2026-07-08 12-42-24

Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
@BelhsanHmida

Copy link
Copy Markdown
Contributor Author

I made some follow-up commits to make the queued payload handling less brittle.

The serializer now preserves plain config/parameter fields by default, and only replaces ORM-backed fields with IDs (future_regressors / past_regressors, sensor / sensor_to_save). I also added a guard so any remaining ORM object in the final queued job payload fails before enqueueing, instead of being pickled into Redis.

Added tests for:

  • plain config/parameter fields surviving serialization
  • regressors and sensors being converted to IDs and restored in the worker session
  • unexpected ORM objects failing loudly

and added some docstrings and comments

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Avoid detached ORM objects in queued train-predict forecasting jobs

4 participants